Skip to content

[0.4.x] libvisual-plugins: Compile and link against OpenGL and GLU in a more portable way (e.g. for macOS) - #278

Merged
hartwork merged 3 commits into
0.4.xfrom
0.4.x-plugins-portable-gl-and-glu
Feb 14, 2023
Merged

hartwork merged 3 commits into
0.4.xfrom
0.4.x-plugins-portable-gl-and-glu

Conversation

@hartwork

Copy link
Copy Markdown
Member

No description provided.

@kaixiong kaixiong left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hartwork, GL actors should be excluded from build if the required OpenGL headers are not found.

@hartwork

hartwork commented Feb 14, 2023 •

Copy link
Copy Markdown
Member Author

@kaixiong we have at least some of that:

# git grep HAVE_OPENGL | grep test
libvisual-plugins/configure.ac:  AS_IF([test "$HAVE_OPENGL" != "yes"], [
libvisual-plugins/configure.ac:  if test "$HAVE_OPENGL" = "yes"; then
libvisual-plugins/configure.ac:  if test "$HAVE_OPENGL" = "yes"; then
libvisual-plugins/configure.ac:  if test "$HAVE_OPENGL" = "yes"; then
libvisual-plugins/configure.ac:  if test "$HAVE_OPENGL" = "yes"; then

If we need, more happy to do more in a new and dedicated pull request.

@hartwork
hartwork requested a review from kaixiong February 14, 2023 15:22
@kaixiong

kaixiong commented Feb 14, 2023 •

Copy link
Copy Markdown
Member

@hartwork, Ah okay. If that's the case, then we do not need the #error when including GL/GLU headers. But I guess it doesn't really matter.

@hartwork

hartwork commented Feb 14, 2023 •

Copy link
Copy Markdown
Member Author

@kaixiong the #error was was made me notice that config.h was not included in two places. It would have failed later anyway, but my vote for keeping it, it's like a better compile-time assertion.

@hartwork
hartwork force-pushed the 0.4.x-plugins-portable-gl-and-glu branch from 4cb73d2 to 0baafec Compare February 14, 2023 15:48
@kaixiong

Copy link
Copy Markdown
Member

@kaixiong the #error was was made me notice that config.h was not included in two places. It would have failed later anyway, but my vote for keeping it, it's like a better compile-time assertion.

@hartwork I think my initial annoyance was more to do with having to complicate the header inclusion logic due to MacOS's unorthodox location. I'm fine with #error either way. I hope that when we do use GLEW or GLAD, we can kill this abomination with a single #include.

@hartwork

Copy link
Copy Markdown
Member Author

@kaixiong we should double check that GLEW pulls in OpenGL via -framework and we should only drop the direct GL includes if we no longer call any GL functions directly. Then...

@hartwork
hartwork merged commit 83445ac into 0.4.x Feb 14, 2023
@hartwork
hartwork deleted the 0.4.x-plugins-portable-gl-and-glu branch February 14, 2023 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants